Skip to content

Add more Prism related tests to cruby-bindings workflow#3609

Merged
tenderlove merged 1 commit into
ruby:mainfrom
Shopify:add-new-cruby-tests
Jul 22, 2025
Merged

Add more Prism related tests to cruby-bindings workflow#3609
tenderlove merged 1 commit into
ruby:mainfrom
Shopify:add-new-cruby-tests

Conversation

@st0012

@st0012 st0012 commented Jul 22, 2025

Copy link
Copy Markdown
Member

This should prevent Prism from breaking CRuby's CI.

@tenderlove

Copy link
Copy Markdown
Member

I'm not 100% sure if we want to do this. In some cases we have a circular dependency where we need to update Prism and CRuby at the same time. Do we merge to CRuby first, then send a PR to Prism? I'm not sure about the order of operations in that case.

This should prevent Prism from breaking CRuby's CI.
@st0012
st0012 force-pushed the add-new-cruby-tests branch from 14594b6 to 1d1ecbf Compare July 22, 2025 15:22
@st0012

st0012 commented Jul 22, 2025

Copy link
Copy Markdown
Member Author

Do we merge to CRuby first, then send a PR to Prism? I'm not sure about the order of operations in that case.

From my (limited) experience working on default gems, yes. There will be a PR including both library change AND CRuby test changes opened on ruby/ruby and gets merged there.
And the relevant changes will be ported back to the library with some local tests.

I think the point of running prism compilation test on CI is to identify the need to do so before we merge a PR.

@tenderlove

Copy link
Copy Markdown
Member

@st0012 makes sense to me!

@tenderlove
tenderlove merged commit dee6c23 into ruby:main Jul 22, 2025
58 checks passed
@kddnewton

Copy link
Copy Markdown
Collaborator

This is fine, thanks @st0012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants